chore(metrics): expose MetricsInterface as return type of single metric and further improve API docs
#3145
+862
−310
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
boring-cyborg
bot
added
internal
pull-request-size
bot
added
the
size/XXL
|
The 2 accepted issues marked by SonarCloud are related to the deprecated method still being used in the unit tests. For now I marked them as such since these tests will be refactored soon when moving the utility to Vitest, and the method will eventually be removed in the next major version. |
leandrodamascena
previously approved these changes
Oct 3, 2024
There was a problem hiding this comment.
Hey @dreamorosi! I just have 2 small comments.
I saw you're changing code in Metrics.ts and types/Metrics.ts and for me looks good. If the tests are green, good to merge.
Co-authored-by: Leandro Damascena <[email protected]>
|
leandrodamascena
approved these changes
Oct 3, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Changes
This PR started out with the goal of modifying the return type of the
Metrics.singleMetric()method to useMetricsInterfaceinstead ofMetrics. This small change allows for easier customization of the utility and avoids a cyclical type reference.While working on this, I realized that the
MetricsInterfacewas out of date and had no documentation whatsoever, which means its API page is empty and there's no contextual documentation in the IDE when customers hover any of the interface or properties.At that point the goal shifted towards making sure the methods and their API docs in the
Metricsclass and theMetricsInterfacewere aligned. In doing so, I also noticed that some of the public methods were somewhat under documented and at least in some cases, there were opportunities to either clarify the purpose of the method or add a code snippet.This ended becoming a seizable PR that further improved on the work started by @am29d in a previous PR and complemented the API docs added in that PR by expanding some of the descriptions and introducing a bunch of links between methods/sections.
For example, now the API docs for the
addMetric()method goes in detail on how to use the method but also links to other relevant parts of the API docs:This work was enabled by @am29d's work on fixing our TypeDoc setup and making sure links are properly rendered & followed. I have applied this type of pattern across the entire Metrics package and also made sure all code samples and wording is consistent across API docs & README.
Finally, while there, I noticed that we had a public method named
throwOnEmptyMetrics()which was severely under documented and that didn't do what I'd have expected it to do. I marked it as deprecated in favor of a newly added methodsetThrowOnEmtpymetrics(enabled: boolean)which is clearer on its purpose and is also aligned with other similar public setter methods in terms of naming.Issue number: closes #3132
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.